Skip to content

Encapsulate all mutable CSP filter settings in a single object #7391

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
labkey-adam merged 2 commits into from
Feb 7, 2026
Merged

Encapsulate all mutable CSP filter settings in a single object #7391

labkey-adam merged 2 commits into from
Feb 7, 2026

Conversation

@labkey-adam
Copy link
Contributor

@labkey-adam labkey-adam commented Feb 4, 2026 •
edited by labkey-tchad
Loading

Rationale

No functional change, this merely groups four per-CSP-filter settings into a single CspSettings object to encapsulate the logic around these settings and set them atomically. This reduces the synchronization required and prevents other threads from seeing a mix of old and new values.

Related Pull Requests

Tasks 📍

  • Manual Testing @labkey-tchad
    • Switch base server URL from http: to https: and back
    • Add and remove allowed external resource hosts
    • Turn the "Disable enforce Content Security Policy" experimental feature on and off
    • After each transition, ensure that the CSP and Reporting-Endpoints are as expected. And no exceptions or errors in the log.

labkey-jeckels
labkey-jeckels approved these changes Feb 5, 2026
View reviewed changes
Copy link
Contributor

@labkey-jeckels labkey-jeckels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't have time to test yet.

@labkey-adam labkey-adam self-assigned this Feb 5, 2026
@labkey-tchad
Copy link
Member

labkey-tchad commented Feb 6, 2026

The "Allowed External Hosts" form throws an error when reshowing after adding or removing a host. Other than that, it's working great.

java.lang.NullPointerException: Cannot invoke "org.labkey.filters.ContentSecurityPolicyFilter$CspFilterSettings.getPolicyTemplate()" because the return value of "org.labkey.filters.ContentSecurityPolicyFilter.getSettings()" is null
	at org.labkey.filters.ContentSecurityPolicyFilter.getMissingSubstitutions(ContentSecurityPolicyFilter.java:440)
	at org.labkey.jsp.compiled.org.labkey.core.admin.addNewExternalSource_jsp._jspService(addNewExternalSource_jsp.java:192)

@labkey-adam
Copy link
Contributor Author

labkey-adam commented Feb 6, 2026

The "Allowed External Hosts" form throws an error when reshowing after adding or removing a host. Other than that, it's working great.

java.lang.NullPointerException: Cannot invoke "org.labkey.filters.ContentSecurityPolicyFilter$CspFilterSettings.getPolicyTemplate()" because the return value of "org.labkey.filters.ContentSecurityPolicyFilter.getSettings()" is null
	at org.labkey.filters.ContentSecurityPolicyFilter.getMissingSubstitutions(ContentSecurityPolicyFilter.java:440)
	at org.labkey.jsp.compiled.org.labkey.core.admin.addNewExternalSource_jsp._jspService(addNewExternalSource_jsp.java:192)

@labkey-tchad Interesting... thanks, I'll take a look.

@labkey-adam
Copy link
Contributor Author

labkey-adam commented Feb 7, 2026

CspResourceHostsTest passed here: https://teamcity.labkey.org/buildConfiguration/bt20/3838650?buildTab=tests

@labkey-adam labkey-adam merged commit ed916c4 into develop Feb 7, 2026
8 checks passed
@labkey-adam labkey-adam deleted the fb_atomic_csp_settings branch February 7, 2026 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@labkey-jeckels labkey-jeckels labkey-jeckels approved these changes

Assignees

@labkey-adam labkey-adam

@labkey-tchad labkey-tchad

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@labkey-adam @labkey-tchad @labkey-jeckels